feat: Wave 1 — search-first skill, confidence thresholds, version manifest by dean0x · Pull Request #138 · dean0x/devflow

dean0x · 2026-03-12T23:02:32Z

Summary

Implements the three independent Wave 1 features from the post-v1.0 roadmap (#137):

Search-first skill (Search-First Skill: Research Before Building #111) — New skill enforcing a 4-phase research loop (Need Analysis → Search → Evaluate → Decide) before writing custom utility code. Added to core-skills plugin and ambient-router BUILD primary skills. 31 skills total (was 30).
Reviewer confidence thresholds (Reviewer Confidence Thresholds: Reduce Noise in Code Review Output #113) — Each review finding now includes a visible confidence score (0-100%). Only ≥80% findings appear in main report sections; 60-79% go to a capped Suggestions section. Adds consolidation rules (group similar issues, skip stylistic preferences, skip unchanged code unless CRITICAL). Fixes synthesizer review glob bug that matched zero reviewer files.
Version manifest (Version manifest for upgrade tracking #91) — Tracks installed version, plugins, scope, and feature flags in manifest.json. Enables upgrade detection during devflow init and shows install status in devflow list. Partial installs merge plugins instead of overwriting. 17 new tests.

Closes #111, closes #113, closes #91

Test plan

npm run build passes — 31 skills, 17 plugins
npm test passes — 208 tests (was 191, +17 manifest tests)
search-first skill distributed to plugins/devflow-core-skills/skills/
Zero regressions on existing tests
Manual: run node dist/cli.js list to verify install status display
Manual: run /code-review on a test branch to verify confidence percentages and suggestions section

New skill enforcing a 4-phase research loop (Need Analysis → Search → Evaluate → Decide) before writing custom utility code. Delegates research to Explore subagent to keep main session context clean. Four decision outcomes: Adopt, Extend, Compose, Build. Added to core-skills plugin and ambient-router BUILD primary skills.

Each review finding now includes a visible confidence score (0-100%). Only findings ≥80% appear in main report sections; 60-79% go to a capped Suggestions section; <60% are dropped entirely. Adds consolidation rules: group similar issues, skip stylistic preferences, skip unchanged code unless CRITICAL. Fixes synthesizer review glob (was *-report.*.md, matched zero files) to *.md with self-exclusion. Adds confidence-aware aggregation with cross-reviewer boosting. Updates Git agent PR comment instructions to only create inline comments for ≥80% confidence findings.

New manifest.json tracks installed version, plugins, scope, and feature flags. Written to devflowDir after successful init. - readManifest/writeManifest/mergeManifestPlugins/detectUpgrade utilities - init.ts: detects upgrade/reinstall via manifest, writes after install - list.ts: shows install status, version, features from manifest - Partial installs merge plugins instead of overwriting - 17 new tests for all manifest operations

dean0x · 2026-03-13T00:01:28Z

src/cli/utils/manifest.ts

+  const manifestPath = path.join(devflowDir, 'manifest.json');
+  try {
+    const content = await fs.readFile(manifestPath, 'utf-8');
+    const data = JSON.parse(content) as ManifestData;


Manifest Validation Gap (BLOCKING)

The readManifest function validates only 3 fields (version, plugins, scope) but casts the full object via as ManifestData. Missing validation for features object, installedAt, and updatedAt.

Risk: Partial/malformed manifests pass validation but cause runtime crashes in list.ts:41 when accessing manifest.features.teams.

Fix: Add full field validation:

if ( \!data.version || \!Array.isArray(data.plugins) || \!data.scope || typeof data.features \!== "object" || data.features === null || typeof data.features.teams \!== "boolean" || typeof data.features.ambient \!== "boolean" || typeof data.features.memory \!== "boolean" || \!data.installedAt || \!data.updatedAt ) { return null; }

Or use Zod schema (project convention per CLAUDE.md).

Confidence: 85% | Consolidated: Reported in 5 reviews (security, architecture, consistency, tests, regression)

Claude Code Review

dean0x · 2026-03-13T00:01:37Z

src/cli/commands/init.ts

+      installedAt: existingManifest?.installedAt ?? now,
+      updatedAt: now,
+    };
+    await writeManifest(devflowDir, manifestData);


Missing Error Handling for writeManifest (BLOCKING)

The writeManifest() call is not wrapped in try/catch. If the write fails (permissions, disk full), the entire init command crashes after plugins are already installed.

Impact: User sees CLI error despite successful installation, confusion about install state.

Fix: Wrap in try/catch with graceful degradation:

try { const installedPluginNames = pluginsToInstall.map(pl => pl.name); const now = new Date().toISOString(); const manifestData = { version, plugins: existingManifest && options.plugin ? mergeManifestPlugins(existingManifest.plugins, installedPluginNames) : installedPluginNames, scope, features: { teams: teamsEnabled, ambient: ambientEnabled, memory: memoryEnabled }, installedAt: existingManifest?.installedAt ?? now, updatedAt: now, }; await writeManifest(devflowDir, manifestData); } catch (error) { p.log.warn(`Could not write manifest: ${error instanceof Error ? error.message : error}`); }

Manifest is a non-critical feature (upgrade tracking only), so write failure should not crash the installer.

Confidence: 82% | Categories: Architecture, Regression

Claude Code Review

dean0x · 2026-03-13T00:02:25Z

Code Review Summary — Blocking & Should-Fix Issues

Blocking Issues (2 inline comments created above)

✅ Manifest Validation Gap (src/cli/utils/manifest.ts:27-30) - FIXED via inline comment

Validates only 3 fields but casts full object; can crash when accessing undefined features properties
Confidence: 85% | Consolidated from 5 reviews

✅ Missing Error Handling (src/cli/commands/init.ts:624) - FIXED via inline comment

writeManifest() not wrapped in try/catch; write failure crashes after successful install
Confidence: 82% | Consolidated from 2 reviews

Documentation Issues (HIGH - Not in PR diff)

⚠️ README.md outdated — These files are not modified in this PR but need updates:

Line 27: Says "30 quality skills" but CLAUDE.md updated to 31
- Fix: Change to "31 quality skills" and note "9 auto-activating core" (was 8)
Lines 109-122: Auto-activating skills table missing search-first entry
- The skill was added to devflow-core-skills plugin.json but README table was not updated
- Confidence: 95% | Category: HIGH

Action: Please update README.md in a follow-up commit to prevent documentation-reality drift.

Should-Fix Issues

📊 Test Coverage Gaps (HIGH Risk)

Missing edge case tests (tests/manifest.test.ts / src/cli/utils/manifest.ts)

readManifest missing test for partial manifest (missing features/installedAt/updatedAt)
compareSemver v-prefix path untested (regex has v? but no test exercises it)
Suggested fixes in review reports (tests.md)

Zero test coverage for integration logic (src/cli/commands/list.ts, src/cli/commands/init.ts)

list.ts manifest reading + scope resolution + feature string logic untested
init.ts conditional plugin merge logic untested (especially risky: existingManifest && options.plugin)
Recommended: Extract pure functions (formatFeatures, resolvePluginList) for testability

Confidence: 85-88% | Category: HIGH

⚡ Performance Opportunity

Sequential I/O in list command (src/cli/commands/list.ts:17-21)

getGitRoot() and readManifest(userDevflowDir) run sequentially but are independent
Could parallelize with Promise.all() for faster execution on slow filesystems
Confidence: 82% | Category: MEDIUM

🎯 Complexity Reduction

Extract formatInstallStatus() helper (src/cli/commands/list.ts:54)

Manifest display logic (127-char ternary) could move to dedicated function
Would reduce action handler nesting and improve readability
Confidence: 80% | Category: MEDIUM

Extract manifest helpers from init.ts (src/cli/commands/init.ts:296-305,611-624)

logUpgradeStatus() and saveManifest() could separate concerns
Keeps action handler from growing further (already 515 lines with 15+ responsibilities)
Confidence: 85% | Category: MEDIUM

Summary by Severity

Category	Count	Details
Blocking	2	Manifest validation, error handling (inlined above)
HIGH	3	README drift (2 items) + test coverage gaps
MEDIUM	4	Performance, complexity extraction (2 files)

Claude Code Review Assistant

Add missing [Unreleased] link reference per keep-a-changelog convention. Points to comparison from v1.4.0 (latest release) to HEAD. Co-Authored-By: Claude <noreply@anthropic.com>

- Update total skill count from 30 to 31 and core auto-activating count from 8 to 9 to reflect the addition of search-first - Add search-first row to auto-activating skills table Co-Authored-By: Claude <noreply@anthropic.com>

…ss-refs - Add report template example for grouped/consolidated findings in reviewer.md so agents know how to format N-occurrence issues - Add cross-reference HTML comments in reviewer.md, synthesizer.md, and code-review.md linking the three locations where the 80% confidence threshold is documented, reducing sync drift risk Resolves: batch-6 (B7, S6) Co-Authored-By: Claude <noreply@anthropic.com>

… I/O - Extract formatFeatures, resolveScope, getPluginInstallStatus, and formatPluginCommands as exported pure functions from list.ts handler - Parallelize getGitRoot() and readManifest() with Promise.all since they are independent I/O operations - Add 14 unit tests in list-logic.test.ts covering all extracted functions Co-Authored-By: Claude <noreply@anthropic.com>

…luginList - Wrap writeManifest call in try/catch so a permissions or disk error after a successful install emits a non-fatal warning instead of crashing the CLI - Use resolvePluginList to encapsulate partial-vs-full install merge decision, replacing inline conditional - Add 5 tests for resolvePluginList covering full install, partial install merge, deduplication, and missing manifest edge cases Co-Authored-By: Claude <noreply@anthropic.com>

Add missing test coverage for manifest utilities: - v-prefixed version strings in compareSemver (via detectUpgrade) - Unparseable installed version in detectUpgrade (symmetric case) - mergeManifestPlugins with empty array boundaries Co-Authored-By: Claude <noreply@anthropic.com>

Remove dead mergeManifestPlugins import from init.ts (now called internally by resolvePluginList in manifest.ts) and align resolveScope return type order to 'user' | 'local' matching codebase convention.

Dean Sharon added 5 commits March 13, 2026 00:42

docs: update changelog and skill count for Wave 1 features

9e316a7

chore: untrack gitignored build artifacts in devflow-specify

cd315f5

dean0x commented Mar 13, 2026

View reviewed changes

Dean Sharon and others added 7 commits March 14, 2026 00:04

docs: add Unreleased comparison link to CHANGELOG.md

7fddd4b

Add missing [Unreleased] link reference per keep-a-changelog convention. Points to comparison from v1.4.0 (latest release) to HEAD. Co-Authored-By: Claude <noreply@anthropic.com>

refactor: simplify resolution fixes

f819ba0

Remove dead mergeManifestPlugins import from init.ts (now called internally by resolvePluginList in manifest.ts) and align resolveScope return type order to 'user' | 'local' matching codebase convention.

dean0x merged commit f4c6faa into main Mar 13, 2026
4 checks passed

dean0x deleted the feat/wave-1 branch March 13, 2026 22:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Wave 1 — search-first skill, confidence thresholds, version manifest#138

feat: Wave 1 — search-first skill, confidence thresholds, version manifest#138
dean0x merged 12 commits intomainfrom
feat/wave-1

dean0x commented Mar 12, 2026

Uh oh!

dean0x Mar 13, 2026

Uh oh!

dean0x Mar 13, 2026

Uh oh!

dean0x commented Mar 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

dean0x commented Mar 12, 2026

Summary

Test plan

Uh oh!

dean0x Mar 13, 2026

Choose a reason for hiding this comment

Manifest Validation Gap (BLOCKING)

Uh oh!

dean0x Mar 13, 2026

Choose a reason for hiding this comment

Missing Error Handling for writeManifest (BLOCKING)

Uh oh!

dean0x commented Mar 13, 2026

Code Review Summary — Blocking & Should-Fix Issues

Blocking Issues (2 inline comments created above)

Documentation Issues (HIGH - Not in PR diff)

Should-Fix Issues

📊 Test Coverage Gaps (HIGH Risk)

⚡ Performance Opportunity

🎯 Complexity Reduction

Summary by Severity

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant